Skip to content

fix(metrics): unify Squads webhook failure types into single metric - #149

Open
hash-envy wants to merge 1 commit into
masterfrom
jm/unify-squad-metrics
Open

fix(metrics): unify Squads webhook failure types into single metric#149
hash-envy wants to merge 1 commit into
masterfrom
jm/unify-squad-metrics

Conversation

@hash-envy

Copy link
Copy Markdown
Contributor

No description provided.

Comment thread jito-bell/src/lib.rs
Err(JitoBellError::Notification(
"All Squads Slack webhooks failed".to_string(),
))
self.epoch_metrics.increment_squads_webhook_failure();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this metric still looks redundant with webhook_errors incremented above for each error in the response?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. It's because of two consequential decisions

  • I made the slack-only squads dispatch BUT I kept the AlertConfig type. This type allows for more than one types of alert channels.
  • As such, it is possible to (and at the time i thought maybe we'd want the option) set multiple slack channels for a message. This means that if one of the slack webhooks is down but another is not, 1 error would fire. Not 2. Not 0.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In practice, we'll never hit this path. But i'd rather not silently ignore one of the channels if we set more than one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants